Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge main into integration/v2 #1480

Merged
merged 68 commits into from
Nov 6, 2023

Conversation

lawrence-forooghian
Copy link
Collaborator

@lawrence-forooghian lawrence-forooghian commented Nov 1, 2023

Other than fixing merge conflicts, the notable changes which bring the recently-added stuff in main in line with the changes from v2 are:

  • Removing mentions of the ably/promises and Realtime.Promise API from the React hooks code and documentation

  • Updating presence message extras test to use the promise-based API

  • Fixing a newly-introduced compilation error calling wsConnection.send in WebSocketTransport (not sure exactly why, but I’m guessing something to do with the upgrade to the ws library in main and perhaps some changes to TypeScript version in v2) — the fix, which uses a type assertion to always pretend we’re always in the Node case, is a bit dodgy but that’s because the signature of ProtocolMessage.serialize is also a bit dodgy (it refers to Buffer even though the code that uses it is also called on web).

The package-lock.json merge conflicts were handled by fixing the package.json merge conflicts then running npm install --lockfile-version 2 using Node 16.20.0. Using Node 18.18.2 seemed to lead to a lockfile that caused an error @esbuild/android-arm not accessible from esbuild when running npm ci in the Node 12 and 14 CI jobs.

lmars and others added 30 commits August 8, 2023 18:08
Signed-off-by: Lewis Marshall <[email protected]>
Support presence message extras
So that wrapper SDKs like Spaces can use the ErrorInfo class.

Signed-off-by: Lewis Marshall <[email protected]>
[SDK-3805] feat: move react-hooks into ably-js
* Be consistent with references to React (not react)
* Provide links to Ably docs so that new users arriving here can discover the Ably documentation and understand concepts
fix: send ably agent as header in react-hooks time ping
docs: add docs for preventing warnings when used with NextJS
refactor!: remove `AblyProvider` options prop
docs: add array -> object hook return values to react migration guide
fix: throw descriptive error when callbacks used with react
@lawrence-forooghian lawrence-forooghian force-pushed the 2023-11-01-merge-main-into-v2 branch from 5d89370 to a757c9f Compare November 3, 2023 12:52
@github-actions github-actions bot temporarily deployed to staging/pull/1480/features November 3, 2023 12:53 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1480/typedoc November 3, 2023 12:55 Inactive
@lawrence-forooghian lawrence-forooghian force-pushed the 2023-11-01-merge-main-into-v2 branch from a757c9f to 4783e31 Compare November 3, 2023 14:10
@github-actions github-actions bot temporarily deployed to staging/pull/1480/features November 3, 2023 14:11 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1480/typedoc November 3, 2023 14:12 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1480/bundle-report November 3, 2023 14:12 Inactive
@lawrence-forooghian lawrence-forooghian force-pushed the 2023-11-01-merge-main-into-v2 branch from 4783e31 to 304be22 Compare November 3, 2023 14:13
@github-actions github-actions bot temporarily deployed to staging/pull/1480/features November 3, 2023 14:13 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1480/typedoc November 3, 2023 14:14 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1480/bundle-report November 3, 2023 14:14 Inactive
This requires us to now explicitly install the browsers (see [1]).

I’m doing this upgrade not because it’s necessary but because NPM tried
to make it for me when resolving a package-lock.json merge conflict in
an upcoming commit. Given that the upgrade involves this CI change,
thought it best to do it as a separate commit so it doesn’t get lost
inside a merge commit.

[1] https://playwright.dev/docs/release-notes#breaking-changes-playwright-no-longer-downloads-browsers-automatically
@lawrence-forooghian lawrence-forooghian force-pushed the 2023-11-01-merge-main-into-v2 branch from 304be22 to d26354d Compare November 3, 2023 15:29
@github-actions github-actions bot temporarily deployed to staging/pull/1480/features November 3, 2023 15:30 Inactive
…-into-v2

Other than fixing merge conflicts, the notable changes which bring the
recently-added stuff in `main` in line with the changes from v2 are:

- Removing mentions of the `ably/promises` and `Realtime.Promise` API
  from the React hooks code and documentation

- Updating presence message extras test to use the promise-based API

- Fixing a newly-introduced compilation error calling
  `wsConnection.send` in WebSocketTransport (not sure exactly why, but
  I’m guessing something to do with the upgrade to the `ws` library in
  `main` and perhaps some changes to TypeScript version in v2) — the fix,
  which uses a type assertion to always pretend we’re always in the Node
  case, is a bit dodgy but that’s because the signature of
  `ProtocolMessage.serialize` is also a bit dodgy (it refers to Buffer
  even though the code that uses it is also called on web).

The package-lock.json merge conflicts were handled by fixing the
package.json merge conflicts then running `npm install --lockfile-version 2`
using Node 16.20.0. Using Node 18.18.2 seemed to lead to a lockfile that
caused an error "@esbuild/android-arm not accessible from esbuild" when
running `npm ci` in the Node 12 and 14 CI jobs.
Copy link
Member

@owenpearson owenpearson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine! Can we make an issue for ProtocolMessage.serialize having an incorrect typing?

@lawrence-forooghian
Copy link
Collaborator Author

Looks fine! Can we make an issue for ProtocolMessage.serialize having an incorrect typing?

Sure — created #1486.

@lawrence-forooghian lawrence-forooghian merged commit 50bb056 into integration/v2 Nov 6, 2023
11 checks passed
@lawrence-forooghian lawrence-forooghian deleted the 2023-11-01-merge-main-into-v2 branch November 6, 2023 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants